fix(vue-router): clear navigation info when a guard aborts navigation - #31364

The gate here was my suggestion, and I missed something when I proposed it. Saving the target path works for the delta, but this line clears incomingRouteParams too, and nothing ever stamps a path onto that. Only history.listen writes currentNavigationInfo.to, and neither setIncomingRouteParams nor changeTab records one, so whenever neither navigation is a history traversal the first disjunct is true and the clear runs unconditionally.

That regresses against main. With a push in flight on a lazy route, a logout ionRouter.replace('/login') cancels it, and the cancelled push wipes the replace's staged params before it gets to use them. The route comes out routerDirection: 'none' with canGoBack() true, where main gives 'root' and false. Without 'root' there's no clearHistory(), so the back button on the login page still walks into the authenticated pages after logout. The tab path loses tab and routerAnimation the same way.

I think stamping the resolved path onto incomingRouteParams, the way history.listen already stamps currentNavigationInfo.to, is the fix that keeps the gate honest for both slots. I haven't tried that one. What I did try is skipping just the params clear on cancelled failures, which gets main's result back with all 20 tests still passing, since 29721's own repro aborts rather than cancels. That leaves an ionRouter.back() cancelled by a push still uncleared though, which is why I'd lean toward the stamp.

Went with the stamp. setIncomingRouteParams now takes an optional target and resolves it, and the gate is two independent checks instead of one answer for both slots. goBack and goForward leave it unset and fall back to the delta's target, which is fine because those are the ones that hand off to history.

One thing I'd like your read on. I kept the target in a separate incomingRouteParamsTo rather than adding a to field to incomingRouteParams. The reason is that the params get spread wholesale onto a RouteInfo at the incomingRouteParams?.id branch, so a field on them would land there too and reach anything reading route info. The cost is two variables to keep in sync, and I had to clear the stamp in all three places the params are cleared. Happy to move it onto the params if you'd rather have one object, since the leak is cosmetic rather than harmful.

67125fe

Nah, keep it as it is. Your reason holds, and I think there's a better version of it. Since handleNavigateBack re-stages a stored RouteInfo as params, a leaked to wouldn't just end up on a RouteInfo, it'd come back out of locationHistory later as a stale target and give you a confidently wrong match at the gate. That's worse than cosmetic.

The shape isn't really what's biting you though, and I left a comment on the gate about that. Folding to onto the object wouldn't fix changeTab either, since it spreads ...incomingRouteParams and would carry the old target forward just the same. What fixes it is having one place that writes the params, so make setIncomingRouteParams the only writer and give the other two a way through it. You'd still have two variables, but only one spot that can desync them, which is the guarantee the single object would've bought you.

Looks like only half of this made it in. Stamping happens in setIncomingRouteParams, but changeTab and handleNavigateBack both set incomingRouteParams directly and just leave whatever stamp was already sitting there. And the stamp is only ever set while a handleNavigate is in flight, which is exactly the case this gate exists for, so the navigation that gets cancelled matches its own stale stamp and wipes the params belonging to the one that replaced it.

I hit this with a global async guard, the session-check kind. Tap a link, then tap the Tab 2 button before the first one finishes, and you get { routerAction: 'push', routerDirection: 'forward', tab: undefined } where main gives { routerAction: 'push', routerDirection: 'none', tab: 'tab2' }. So the tab slides in like a forward push instead of swapping, and since tab is gone it takes the isPushed branch and picks up pushedByRoute from the tab you just left, which means the back button over on tab 2 sends you into tab 1's stack.

It's wider than I expected, too. On a 400ms guard I tried gaps from 10ms all the way up to about 400ms and every one of them did it, main was right on all of them. Makes sense in hindsight, the first navigation starts its guard first so it always reports the cancellation before the second one finishes. It does need a guard though. With plain lazy routes the tab's chunk is already cached by the time changeTab can even take this branch, so the tab always wins and nothing breaks, which is probably why the specs don't catch it.

The handleNavigateBack path looks like the same shape, I didn't manage a clean repro of that one. Both of them know their target though, so changeTab could stamp its pathname and handleNavigateBack could just null the stamp before router.back() so it falls back to the delta the way your comment says. Honestly, making setIncomingRouteParams the only thing that writes the params would save us from this coming back. The incomingRouteParamsTo doc needs a tweak either way, it says the back and forward helpers leave it undefined where those two leave it stale.

150ecb2

An async guard that throws instead of returning false never gets here at all, so the original bug still reproduces. Internally triggerError hands back a rejected promise, so the .then that would call triggerAfterEach never runs and the .catch(noop) on the end eats it. No failure object, no afterEach.

You don't need a deliberate throw either, an await on a session check that rejects does it. On /profile, tap back, guard throws, then tap a link to /settings: the URL says /settings but Ionic reports /home with a pop, and the stack collapses down to just Home, so Settings never mounts and Profile gets destroyed. Same on main so nothing regressed, it's just not covered.

Different thing from FW-7699, that one's the guard-returning-a-location case. I think a router.onError doing the same two gated clears would close it, and onError doesn't swallow the error so it wouldn't change any navigation outcomes. Feels cheap enough to do here, but I'm fine with a card if you'd rather keep this PR tight.

I don't think anything actually covers the case the stamp was added for, where the stamp is set and matches the navigation that failed. Every spec that fails a navigation fails a router.back(), a goBack or a handleNavigateBack, and none of those stamp anything. This one's the mismatch side, the replacing navigation had already overwritten the stamp by the time the failure came through. I swapped the matching branch for a hard false to check, and all 21 still passed.

Something like this would cover it, and it does fail on main: handleNavigate('/blocked', 'replace', 'root') blocked by a guard, then a plain router.push('/other'). Main leaks the replace/root onto /other, which calls clearHistory() and wipes the back stack. This branch gives you push/forward.

Found two things that can never match here, so the params just don't get cleared. Both leak on main too so neither is a regression, but they aren't mentioned anywhere and you can hit both without any racing.

Calling router.resolve() doesn't follow redirect: records, while afterEach gives you the location after the redirect. The plain tabs setup is enough: with { path: '/tabs', redirect: '/tabs/tab1' } and a guard keeping logged-out users off the tabs, an ionRouter.replace('/tabs', 'root') stamps /tabs while afterEach says /tabs/tab1, so the replace/root leaks onto whatever gets tapped next. And root calls clearHistory(), so that wipes the back stack. Not FW-7699 though, afterEach does run for this one.

The second one's narrower, but less narrow than I first thought. Every non-push action goes out through router.replace, and the string and object branches of resolve encode the query differently, so a space is enough to break it. With default-href="/search?q=hello world" the stamp keeps the space, afterEach reports ?q=hello+world, and the pop/back params leak the same way. Plain push is fine, and ?q=a/b and everything else I tried matched on both sides.

One thing that's fine, so you don't go chasing it: the delta gate. A redirect rewrites the history entry, so pushing /old leaves /new sitting in history, and a back into it reports /new on both sides.

Nah, keep it as it is. Your reason holds, and I think there's a better version of it. Since handleNavigateBack re-stages a stored RouteInfo as params, a leaked to wouldn't just end up on a RouteInfo, it'd come back out of locationHistory later as a stale target and give you a confidently wrong match at the gate. That's worse than cosmetic.

The shape isn't really what's biting you though, and I left a comment on the gate about that. Folding to onto the object wouldn't fix changeTab either, since it spreads ...incomingRouteParams and would carry the old target forward just the same. What fixes it is having one place that writes the params, so make setIncomingRouteParams the only writer and give the other two a way through it. You'd still have two variables, but only one spot that can desync them, which is the guarantee the single object would've bought you.